Skip to content

Followed Resolved Type Ref instead of panicking #354

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Dec 22, 2016
Merged

Followed Resolved Type Ref instead of panicking #354

merged 2 commits into from
Dec 22, 2016

Conversation

Nashenas88
Copy link
Contributor

This could fix #353, but I would really want everyone to review in case I'm missing something important. The compilation continues for the code I'm working with. Also, should I add a regression test for this? I could simply use the code in the referenced issue.

@fitzgen
Copy link
Member

fitzgen commented Dec 21, 2016

r? @emilio

I feel like we need a variant of the various resolve_foo context methods that automatically follows resolved type refs (and maybe aliases as well?). We can spot fix issues like this one, but it would be nice if we had a more base solution for the 99% case.

@fitzgen
Copy link
Member

fitzgen commented Dec 21, 2016

Also, thanks for the pull request, @Nashenas88 :)

@fitzgen
Copy link
Member

fitzgen commented Dec 21, 2016

Also, should I add a regression test for this? I could simply use the code in the referenced issue.

Yes, that would be great.

@emilio
Copy link
Contributor

emilio commented Dec 22, 2016

I feel like we need a variant of the various resolve_foo context methods that automatically follows resolved type refs (and maybe aliases as well?). We can spot fix issues like this one, but it would be nice if we had a more base solution for the 99% case.

We do have that, which is the canonical_type thing. We could/should use that here. We need to publish a new version, so let's merge this and I'll get that cleaned up before publishing.

@bors-servo r+

Thanks for the PR @Nashenas88! :)

@bors-servo
Copy link

📌 Commit a336b01 has been approved by emilio

@bors-servo
Copy link

⌛ Testing commit a336b01 with merge 4472e0a...

bors-servo pushed a commit that referenced this pull request Dec 22, 2016
Auto merge of #354 - Nashenas88:master, r=emilio
4472e0a 
Followed Resolved Type Ref instead of panicking

This could fix #353, but I would really want everyone to review in case I'm missing something important. The compilation continues for the code I'm working with. Also, should I add a regression test for this? I could simply use the code in the referenced issue.
@bors-servo
Copy link

☀️ Test successful - status-travis

@bors-servo bors-servo merged commit a336b01 into rust-lang:master Dec 22, 2016
@emilio emilio mentioned this pull request Dec 22, 2016
Merged
bors-servo pushed a commit that referenced this pull request Dec 23, 2016
Auto merge of #357 - emilio:auto, r=fitzgen
cbc0304 
Multiple cleanups and fix for #355

Fixes #355

Also improves the output of #354.

r? @fitzgen
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@emilio emilio

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

libbindgen fails with "How?"
5 participants
@Nashenas88 @fitzgen @emilio @bors-servo @highfive